Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Document Picker crash on IOS #3581

Merged
merged 1 commit into from
Jun 14, 2021
Merged

fix: Document Picker crash on IOS #3581

merged 1 commit into from
Jun 14, 2021

Conversation

parasharrajat
Copy link
Member

@parasharrajat parasharrajat commented Jun 14, 2021 •
edited
Loading

Details

We had to put a delay between modal is closed and picker opens the document chooser.

Fixed Issues

Fixes #3543

Tests | QA Steps

  1. Open any conversation on E.cash mobile app.
  2. click Add attachment from plus icon.
  3. Select the add attachment option and

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

ios-picker.mp4

Android

1623693284878.mp4

@parasharrajat parasharrajat requested a review from a team as a code owner June 14, 2021 18:00
@MelvinBot MelvinBot requested review from ctkochan22 and removed request for a team June 14, 2021 18:01
Julesssss
Julesssss approved these changes Jun 14, 2021
View reviewed changes
Copy link
Contributor

@Julesssss Julesssss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to suggest reducing this to 100ms, as 200ms is quite noticeable as a user. But that's how we got into this mess in the first place, so I won't block on that.

@parasharrajat
Copy link
Member Author

I checked that 100ms does not work. but I am looking into other factors which cause this setTimout. No luck so far.

@roryabraham
Copy link
Contributor

Great job finding the solution here @parasharrajat! 🎉

Do you know if there is an issue in the react-native-modal repo for this? It seems that onModalHide is being executed before the modal is completely closed, which it should not. If there's no issue, should we consider opening one?

@parasharrajat
Copy link
Member Author

parasharrajat commented Jun 14, 2021 •
edited
Loading

They do seem to fix it here by calling it after state update. https://github.com/react-native-modal/react-native-modal/blob/b580105f4b2b4e1163bed1228f241b16ea17dacb/src/modal.tsx#L697. but I don't think it can be improved further but nobody knows. It could be an issue with the React-native modal. Not sure about opening another issue. there are a couple of closed ones there.

@parasharrajat
Copy link
Member Author

@roryabraham it seems we missed the Slack Webhook Url Env for Gh actions.

@roryabraham
Copy link
Contributor

roryabraham commented Jun 14, 2021 •
edited
Loading

@roryabraham it seems we missed the Slack Webhook Url Env for Gh actions.

This is a known issue for GH Actions where your repo secrets randomly disappear for some workflow runs for no reason. 😠

@ctkochan22
Copy link
Contributor

Confirmed with @roryabraham that we are good to merge

@ctkochan22 ctkochan22 merged commit 5a04371 into Expensify:main Jun 14, 2021
github-actions bot pushed a commit that referenced this pull request Jun 14, 2021
@ctkochan22
Merge pull request #3581 from parasharrajat/attachment-picker
258744f 
fix: Document Picker crash on IOS
(cherry picked from commit 5a04371)
This was referenced Jun 14, 2021
Merged
Closed
@OSBotify
Copy link
Contributor

🚀 Cherry-picked to staging in version: 1.0.68-4🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify OSBotify mentioned this pull request Jun 15, 2021
Closed
@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.68-4🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.68-5🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.73-3🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

This was referenced Jun 25, 2021
Closed
Closed
@OSBotify OSBotify mentioned this pull request Jul 7, 2021
Closed
57 tasks
@parasharrajat parasharrajat mentioned this pull request Jul 12, 2021
Merged
5 tasks
This was referenced Jul 12, 2021
Closed
Closed
@parasharrajat parasharrajat deleted the attachment-picker branch November 4, 2022 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Julesssss Julesssss Julesssss approved these changes

@ctkochan22 ctkochan22 ctkochan22 approved these changes

Assignees

@parasharrajat parasharrajat

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[HOLD for payment 21 June] iOS - Attachment picker flickers and unable to select for choose document
5 participants
@parasharrajat @roryabraham @ctkochan22 @OSBotify @Julesssss